Implement the Getis-Ord Gi* statistic in hotspots()#2803
Merged
Conversation
hotspots() advertised Gi* but computed a local-mean z-score: it convolved the raster with a normalized kernel, then z-scored that local mean against the global mean and std. That dropped the neighborhood weight sum, the squared weight sum, the sample count n, and the variance adjustment term, so the classified output was wrong. Replace it with the real Gi* z-score on all four backends (numpy, cupy, dask+numpy, dask+cupy): numerator: sum_j w_ij x_j - Xbar * W_i denominator: S * sqrt((n * W2_i - W_i^2) / (n - 1)) W_i and W2_i come from convolving the validity mask with the kernel and the squared kernel, so raster edges and interior NaN cells are left out of the neighborhood sums. n, Xbar (mean), and S (population std) are global scalars over the valid cells. Tests check the z-scores against a hand-computed reference for a small known window (binary and weighted kernels), NaN exclusion, the n<2 guard, and numpy/dask parity. Updates the docstring and the focal.rst NaN-handling note.
brendancol
commented
Jun 1, 2026
Contributor
Author
brendancol
left a comment
There was a problem hiding this comment.
PR Review: Implement the Getis-Ord Gi* statistic in hotspots()
Blockers (must fix before merge)
None.
Suggestions (should fix, not blocking)
- focal.py:1296
_gistar_zscoredoes its arithmetic in float32 (whatever the convolution returned) and casts at the end. For largenand large weights,n * sq_weight_sum - weight_sum^2is a difference of large numbers and can lose precision in float32. The hand-reference tests run in float64 and pass at rtol 1e-3 to 1e-4, so the tolerance hides it. Accumulating the variance term in float64 before the sqrt would be safer. Not blocking: the z thresholds (1.65/1.96/2.58) are coarse, so float32 rounding rarely flips a band.
Nits (optional improvements)
- focal.py:1305
_gistar_convolutions_numpyis numpy-only and the tests import it directly, while the dask path inlines the same three convolutions in_hotspots_chunk. If those two spellings drift, the tests only catch the numpy one. - The dask+numpy and dask+cupy paths each compute n/mean/std with three reductions in one
da.compute(...). It is a single pass, but the mask.sum()andnanmean/nanstdrebuild the NaN mask three times. Minor; not worth changing.
What looks good
- Gi* z-scores checked against an independent hand-computed reference on a known window, for binary and weighted kernels, not just the classification banding.
- NaN cells drop out of the neighborhood sums and the global stats via a validity mask convolved with the kernel and the squared kernel. That is the correct treatment.
- All four backends share the same
_gistar_zscoremath through the existing dispatch. numpy, cupy, dask+numpy, and dask+cupy all pass on a real GPU here. n < 2and zero-std guards have clear errors.- The numpy/dask edge divergence is the documented boundary='nan' artifact; the tests compare the interior, which matches the existing dask+cupy hotspots test.
Checklist
- Algorithm matches the reference formula
- All backends agree on the interior
- NaN handling is correct
- Edge cases covered (NaN, constant surface, n<2, tiny raster)
- Dask chunk boundaries handled (depth = kernel radius)
- No premature materialization beyond the documented scalar pass
- Benchmark: existing hotspots benchmark still applies; no new function
- README feature matrix: no change needed
- Docstrings present and accurate (formula added)
Address review: n * W2 - W^2 is a difference of large numbers that loses precision in float32 for big rasters or large kernel weights. Promote the weight sums and numerator to float64 before the sqrt, then cast the z-score back to float32. Cells and bands are unchanged on the existing tests.
brendancol
commented
Jun 1, 2026
Contributor
Author
brendancol
left a comment
There was a problem hiding this comment.
Follow-up review (after b36e43f)
Disposition of the first-pass findings:
- Suggestion (float32 variance precision, focal.py:1296): fixed in b36e43f. The weight sums and numerator are promoted to float64 before the sqrt, then the z-score is cast back to float32.
n * W2 - W^2no longer loses precision for large rasters or weights. Banding is unchanged on the existing tests; interior z-scores now match the float64 reference to ~2e-7 (the residual is from the float32 convolution inputs, not the variance term). - Nit (numpy-only
_gistar_convolutions_numpyvs the inlined dask convolutions): dismissed.test_hotspots_gistar_dask_matches_numpy_fullruns the dask inline path against the numpy reference on the full interior, so drift between the two spellings would fail a test. - Nit (three reductions rebuild the NaN mask): dismissed, as flagged in the first pass. It is a single
da.computepass and the cost is negligible.
Re-ran test_focal, test_emerging_hotspots, test_validation, test_dask_laziness: 296 passed, including the GPU hotspots paths on a real device. No new findings.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2767
hotspots()claimed to compute the Getis-Ord Gi* statistic but computed a local-mean z-score instead: it convolved the raster with a normalized kernel, then z-scored that local mean against the global mean and std. That formula drops the neighborhood weight sum, the squared weight sum, the sample count n, and the variance adjustment term, so the classified output was statistically wrong.What changed:
(sum_j w_ij x_j - Xbar * W_i) / (S * sqrt((n * W2_i - W_i^2) / (n - 1))).W_iandW2_iby convolving the validity mask with the kernel and the squared kernel, so raster edges and interior NaN cells drop out of the neighborhood sums.n,Xbar, andS(population std) are global scalars over the valid cells.n < 2(the variance term divides byn - 1).focal.rstNaN-handling note.Backends: numpy, cupy, dask+numpy, dask+cupy all use the same Gi* math through the existing
ArrayTypeFunctionMappingdispatch.Test plan:
W_i != W2_i.n < 2raises.pytest xrspatial/tests/test_focal.py(full file), plustest_emerging_hotspots.py,test_convolution.py,test_validation.py,test_dask_laziness.pyall pass.Note:
emerging_hotspots()carries the same local-mean z-score bug in its own code path. It is out of scope here (one fix per PR) and tracked in #2804.